Add prompt id patch + eval pull#444
Conversation
WalkthroughAdds a new Cobra-based eval CLI with create and pull subcommands, associated API helpers and types, interactive/TTY-aware prompts, file output to src/evals/*.ts, and flags. Updates Vercel AI bundler to inject telemetry setup with prompt metadata and prompt stringification for multiple AI functions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as eval CLI
participant API as Cloud API
participant FS as Filesystem
rect rgb(235,245,255)
note over User,CLI: eval create (TTY-aware)
User->>CLI: eval create [--force] [-d DIR] [name desc]
CLI->>CLI: Determine type (template/generative)
alt template
CLI->>CLI: Gather name/description (args or prompts)
else generative
CLI->>CLI: Use generative path
end
CLI->>User: Confirm (skip if --force)
alt confirmed
alt template
CLI->>API: CreateTemplateEvaluation(projectId, name, desc)
else generative
CLI->>API: CreateGenerativeEvaluation(projectId)
end
API-->>CLI: evalId
CLI->>API: PullEvaluation(evalId)
API-->>CLI: { code, id, name, description }
CLI->>FS: Write src/evals/<Name>.ts
CLI-->>User: Output code and success info
else canceled
CLI-->>User: Abort
end
end
sequenceDiagram
participant User
participant CLI as eval CLI
participant API as Cloud API
participant FS as Filesystem
note over User,CLI: eval pull
User->>CLI: eval pull --id <evalId> [-d DIR]
CLI->>API: PullEvaluation(evalId)
API-->>CLI: { code, id, name, description }
CLI->>FS: Write src/evals/<Name>.ts
CLI-->>User: Output code
sequenceDiagram
participant Caller
participant Patch as Before Patch
participant AI as Vercel AI fn
Caller->>Patch: call generateText(...) with _args[0]=opts
Patch->>Patch: opts = { ..._args[0] }
Patch->>Patch: metadata = { promptId: opts.prompt.id }
Patch->>Patch: opts.experimental_telemetry = { isEnabled: true, metadata }
Patch->>Patch: opts.prompt = opts.prompt.toString()
Patch->>AI: proceed with _args[0] = opts
AI-->>Caller: result/stream
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| opts.experimental_telemetry = { isEnabled: true , metadata: metadata }; | ||
| opts.prompt = opts.prompt.toString(); | ||
| _args[0] = opts; | ||
| `) |
There was a problem hiding this comment.
Bug: Telemetry Patch Fails Without Prompt Validation
The telemetry patch accesses opts.prompt.id and calls opts.prompt.toString() without checking if opts.prompt exists or has the expected structure. This can lead to runtime errors, especially for Vercel AI functions like embed that may not include a prompt object in their arguments.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
cmd/eval.go (4)
221-224: Protect against accidental overwrite (create).Currently overwrites without warning. Prompt on TTY unless
--forceis set.- filePath := filepath.Join(evalsDir, filename) - if err := os.WriteFile(filePath, []byte(evalObj.Code), 0644); err != nil { + filePath := filepath.Join(evalsDir, filename) + if _, err := os.Stat(filePath); err == nil && tui.HasTTY && !force { + if !tui.Ask(logger, fmt.Sprintf("File %s exists. Overwrite?", filePath), false) { + tui.ShowWarning("cancelled") + return + } + } + if err := os.WriteFile(filePath, []byte(evalObj.Code), 0644); err != nil { errsystem.New(errsystem.ErrOpenFile, err, errsystem.WithUserMessage("Failed to write evaluation code to file")).ShowErrorAndExit() }
273-276: Protect against accidental overwrite (pull).Prompt before overwriting, mirroring create behavior.
- filename := evalObj.Name + ".ts" + safeName := filepath.Base(evalObj.Name) + safeName = strings.TrimSpace(safeName) + if safeName == "" { safeName = "evaluation" } + safeName = strings.Map(func(r rune) rune { + switch r { + case '/', '\\', ':', '*', '?', '"', '<', '>', '|': + return '_' + default: + return r + } + }, safeName) + filename := safeName + ".ts" evalsDir := filepath.Join(dir, "src", "evals") @@ - filePath := filepath.Join(evalsDir, filename) - if err := os.WriteFile(filePath, []byte(evalObj.Code), 0644); err != nil { + filePath := filepath.Join(evalsDir, filename) + if _, err := os.Stat(filePath); err == nil && tui.HasTTY { + if !tui.Ask(logger, fmt.Sprintf("File %s exists. Overwrite?", filePath), false) { + tui.ShowWarning("cancelled") + return + } + } + if err := os.WriteFile(filePath, []byte(evalObj.Code), 0644); err != nil { errsystem.New(errsystem.ErrOpenFile, err, errsystem.WithUserMessage("Failed to write evaluation code to file")).ShowErrorAndExit() }Note: Ensure
"strings"is imported (see earlier import diff).Also applies to: 282-285
122-127: Avoid shadowing thecontextimport.
context := project.EnsureProject(...)shadows thecontextpackage and harms readability.Consider renaming to
projCtxorpctxand updating subsequent uses.
210-215: Nil-guardevalObjafter spinner.If an unexpected error path returns without exiting,
evalObjcould be nil.spinner.New().Title("Creating evaluation...").Action(action).Run() // Write code to file - filename := evalObj.Name + ".ts" + if evalObj == nil { + errsystem.New(errsystem.ErrApiRequest, nil, errsystem.WithUserMessage("Evaluation payload was empty")).ShowErrorAndExit() + } + filename := evalObj.Name + ".ts"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cmd/eval.go(1 hunks)internal/bundler/vercel_ai.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/eval.go (4)
internal/util/api.go (1)
NewAPIClient(70-78)internal/project/project.go (1)
EnsureProject(586-610)internal/errsystem/errorcodes.go (3)
ErrApiRequest(21-24)ErrCreateDirectory(61-64)ErrOpenFile(73-76)internal/errsystem/errsystem.go (1)
WithUserMessage(70-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Cursor Bugbot
- GitHub Check: Build and Test (macos-latest)
- GitHub Check: Test CLI Upgrade Path (windows-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
internal/bundler/vercel_ai.go (1)
34-40: experimental_telemetry is correct — no changes required.
Docs confirm per-call experimental_telemetry with isEnabled and metadata (plus optional functionId/recordInputs/recordOutputs) is the supported shape for generateText/streamText.
| // Write code to file | ||
| filename := evalObj.Name + ".ts" | ||
| evalsDir := filepath.Join(dir, "src", "evals") | ||
|
|
There was a problem hiding this comment.
Sanitize file name to prevent path traversal and invalid chars.
evalObj.Name + ".ts" can escape src/evals or include illegal characters.
Apply this diff (imports added below):
- filename := evalObj.Name + ".ts"
- evalsDir := filepath.Join(dir, "src", "evals")
+ safeName := filepath.Base(evalObj.Name)
+ safeName = strings.TrimSpace(safeName)
+ if safeName == "" {
+ safeName = "evaluation"
+ }
+ // replace path separators and disallowed chars
+ safeName = strings.Map(func(r rune) rune {
+ switch r {
+ case '/', '\\', ':', '*', '?', '"', '<', '>', '|':
+ return '_'
+ default:
+ return r
+ }
+ }, safeName)
+ filename := safeName + ".ts"
+ evalsDir := filepath.Join(dir, "src", "evals")Add import:
import (
"context"
"fmt"
"os"
"os/signal"
"path/filepath"
"syscall"
+ "strings"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Write code to file | |
| filename := evalObj.Name + ".ts" | |
| evalsDir := filepath.Join(dir, "src", "evals") | |
| import ( | |
| "context" | |
| "fmt" | |
| "os" | |
| "os/signal" | |
| "path/filepath" | |
| "syscall" | |
| "strings" | |
| ) |
| // Write code to file | |
| filename := evalObj.Name + ".ts" | |
| evalsDir := filepath.Join(dir, "src", "evals") | |
| // Write code to file | |
| safeName := filepath.Base(evalObj.Name) | |
| safeName = strings.TrimSpace(safeName) | |
| if safeName == "" { | |
| safeName = "evaluation" | |
| } | |
| // replace path separators and disallowed chars | |
| safeName = strings.Map(func(r rune) rune { | |
| switch r { | |
| case '/', '\\', ':', '*', '?', '"', '<', '>', '|': | |
| return '_' | |
| default: | |
| return r | |
| } | |
| }, safeName) | |
| filename := safeName + ".ts" | |
| evalsDir := filepath.Join(dir, "src", "evals") |
| func init() { | ||
| rootCmd.AddCommand(evalCmd) | ||
|
|
||
| evalCreateCmd.Flags().Bool("force", !hasTTY, "Don't prompt for confirmation") |
There was a problem hiding this comment.
Fix compile error: undefined hasTTY.
Use the exported tui.HasTTY.
- evalCreateCmd.Flags().Bool("force", !hasTTY, "Don't prompt for confirmation")
+ evalCreateCmd.Flags().Bool("force", !tui.HasTTY, "Don't prompt for confirmation")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| evalCreateCmd.Flags().Bool("force", !hasTTY, "Don't prompt for confirmation") | |
| evalCreateCmd.Flags().Bool("force", !tui.HasTTY, "Don't prompt for confirmation") |
🤖 Prompt for AI Agents
In cmd/eval.go around line 297 the code references an undefined identifier
`hasTTY`; replace it with the exported `tui.HasTTY` (i.e., use tui.HasTTY when
setting the default for the "force" flag) and ensure the package `tui` is
imported at the top of the file if not already present. Ensure the flag line
becomes evalCreateCmd.Flags().Bool("force", !tui.HasTTY, "Don't prompt for
confirmation") and run `go build` to confirm compilation.
| var vercelTelemetryPatch = generateJSArgsPatch(0, ``) + fmt.Sprintf(` | ||
| const opts = {...(_args[0] ?? {}) }; | ||
| const metadata = { promptId: opts.prompt.id }; | ||
| opts.experimental_telemetry = { isEnabled: true , metadata: metadata }; | ||
| opts.prompt = opts.prompt.toString(); | ||
| _args[0] = opts; | ||
| `) |
There was a problem hiding this comment.
Guard against undefined opts.prompt and avoid breaking non-text APIs (embed/embedMany).
Directly accessing opts.prompt.id and calling .toString() will throw when prompt is absent (not provided for embed/embedMany) or not an object. Also, don’t clobber existing experimental_telemetry.
Apply this diff:
- var vercelTelemetryPatch = generateJSArgsPatch(0, ``) + fmt.Sprintf(`
- const opts = {...(_args[0] ?? {}) };
- const metadata = { promptId: opts.prompt.id };
- opts.experimental_telemetry = { isEnabled: true , metadata: metadata };
- opts.prompt = opts.prompt.toString();
- _args[0] = opts;
- `)
+ var vercelTelemetryPatch = generateJSArgsPatch(0, ``) + fmt.Sprintf(`
+ const opts = { ...(_args[0] ?? {}) };
+ const p = opts.prompt;
+ const metadata = {};
+ if (p && typeof p === 'object' && 'id' in p) {
+ metadata.promptId = p.id;
+ }
+ opts.experimental_telemetry = { ...(opts.experimental_telemetry ?? {}), isEnabled: true, metadata };
+ if (typeof p === 'string') {
+ // keep as-is
+ } else if (p && typeof p.toString === 'function') {
+ opts.prompt = p.toString();
+ }
+ _args[0] = opts;
+ `)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var vercelTelemetryPatch = generateJSArgsPatch(0, ``) + fmt.Sprintf(` | |
| const opts = {...(_args[0] ?? {}) }; | |
| const metadata = { promptId: opts.prompt.id }; | |
| opts.experimental_telemetry = { isEnabled: true , metadata: metadata }; | |
| opts.prompt = opts.prompt.toString(); | |
| _args[0] = opts; | |
| `) | |
| var vercelTelemetryPatch = generateJSArgsPatch(0, ``) + fmt.Sprintf(` | |
| const opts = { ...(_args[0] ?? {}) }; | |
| const p = opts.prompt; | |
| const metadata = {}; | |
| if (p && typeof p === 'object' && 'id' in p) { | |
| metadata.promptId = p.id; | |
| } | |
| opts.experimental_telemetry = { ...(opts.experimental_telemetry ?? {}), isEnabled: true, metadata }; | |
| if (typeof p === 'string') { | |
| // keep as-is | |
| } else if (p && typeof p.toString === 'function') { | |
| opts.prompt = p.toString(); | |
| } | |
| _args[0] = opts; | |
| `) |
🤖 Prompt for AI Agents
In internal/bundler/vercel_ai.go around lines 34 to 40, the injected JS assumes
opts.prompt exists and is an object and unconditionally overwrites
opts.experimental_telemetry, which breaks embed/embedMany and non-object
prompts; fix by guarding access: check that opts.prompt is defined and typeof
opts.prompt === 'object' before reading prompt.id and calling toString(), only
set metadata when an id exists, and if prompt is a primitive use
String(opts.prompt) instead of .toString() on undefined; also merge into any
existing opts.experimental_telemetry (e.g., opts.experimental_telemetry = {
...(opts.experimental_telemetry||{}), isEnabled: true, metadata }) rather than
clobbering it so existing telemetry fields are preserved.
1 participant
Summary by CodeRabbit
New Features
Chores